-
Notifications
You must be signed in to change notification settings - Fork 714
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug: fix missing cs_main lock in AppInitMain and LoadExternalBlockFile #2728
Conversation
22ec77f
to
eca2977
Compare
and in calls to TxToJSON/blockToJSON from rest.cpp Add more locking annotations and assertions.
eca2977
to
5ca8e22
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 5ca8e22
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 5ca8e22
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code ACK 5ca8e22
Left few comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
k, ACK 5ca8e22 and merging..
…JSON and TxToJSON 5e0d308 rpc: Remove cs_main lock from TxToJSON (random-zebra) fe7b1cf rpc: Remove cs_main lock from blockToJSON and blockHeaderToJSON (João Barbosa) Pull request description: Follow-up to #2728. This pull makes blockToJSON, blockheaderToJSON, and TxToJSON free of cs_main locks by passing the chainTip and using `GetAncestor` to check if a block is part of the active chain (as per bitcoin#12151). ACKs for top commit: Fuzzbawls: ACK 5e0d308 furszy: utACK 5e0d308 and merging.. Tree-SHA512: 800b45962489a66d45044618c1a40f39cd498efd7eb573db5cf0d6a07edf9fa68ca0e69a3a599f133802daddb1d2efd4ecb80d0bbf1fd17f8b4cef67ee7dd410
#2715 introduced a locking assertion in
InsertBlockIndex
, which is currently failing (with debug enabled) becausecs_main
is not held during the blockindex load inAppInitMain
(bitcoin@c651df8).Also,
LookupBlockIndex
is being called without cs_main inLoadExternalBlockFile
(bitcoin@f814a3e).Finally there's a couple of missing locks in rest.cpp (which can be removed later, porting bitcoin#12151).